Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a UX workaround for rope/pyls rename issue #127

Merged
merged 3 commits into from
Jan 5, 2020

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Jan 4, 2020

Implement a UX workaround for rope/pyls rename issue for the case of syntax errors in the source code.

In #115 an issue with rename for Python (when using pyls) was identified:
rename was failing with an obscure message when the source code could not be parsed correctly by rope (due to a user's syntax error).

This workaround detects such a condition using diagnostics feature provides a nice error message to the user (UX first).

References

#115

Code changes

ICommandContext now includes VirtualEditor. Diagnostics feature got a new, public database of diagnostics (diagnostics_db: IEditorDiagnostic[]) which will be also used to implement the diagnostics Panel widget (#42).

User-facing changes

Screenshot from 2020-01-04 22-02-53

Note: the error is in the 4th cell (1-based indexing) because above the code cells there are two markdown cells; we could change the code to display the ordinal number for code cells only (but not sure how it would work in future if we decide to analyze markdown cells too).

Backwards-incompatible changes

None

Chores

  • linted
  • tested
  • documented
  • changelog entry

for the case of syntax errors in the source code.
@krassowski
Copy link
Member Author

Note: even though I filled an issue in pyls (and will probably fill another in rope), even if we contribute a fix upstream, we would like to have a wrapper on this case of our own making - the concept of cell does not exist in rope/pyls so we would not be able to direct user to a specific cell (as we do here).

as this require user to read a longer text
@krassowski krassowski changed the base branch from rename_for_notebooks to master January 5, 2020 23:00
@krassowski krassowski merged commit 6d9fd54 into master Jan 5, 2020
@krassowski krassowski deleted the rename_syntax_error_ux_workaround branch March 14, 2020 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant